Skip to content

Clean includes#145

Merged
ktf merged 3 commits into
AliceO2Group:devfrom
MohammadAlTurany:clean_includes
Aug 19, 2016
Merged

Clean includes#145
ktf merged 3 commits into
AliceO2Group:devfrom
MohammadAlTurany:clean_includes

Conversation

@MohammadAlTurany

Copy link
Copy Markdown
Contributor

Remove the Alfa_DIR from the cmake files and update the installation instructions (GH-#30)

@ktf

ktf commented Aug 18, 2016

Copy link
Copy Markdown
Member

I can merge this. One question, though. Do we want to squash the commits into one when we merge? Github now allows for this and it helps keeping the history cleaner, IMHO. Any @MohammadAlTurany, @pzhristov , @dberzano, @Barthelemy any opinions?

@dberzano

Copy link
Copy Markdown
Contributor

I prefer squashing. One PR == one feature.

If one wants to see the detailed commits and comments, she can always click on the PR (which is automatically referenced by GitHub in the squashed commit) and look at the history.

Moreover we get rid of "merge commits".

As a "disadvantage", whoever works on master and creates PRs from her master (instead of creating it from a separate branch) will not be able to simply pull from the upstream master once the PR is merged, but a git fetch && git reset --hard upstream/master with a subsequent git push -f myrepo will be required.

@ktf

ktf commented Aug 18, 2016 via email

Copy link
Copy Markdown
Member

@MohammadAlTurany

Copy link
Copy Markdown
Contributor Author

Hi,
Fine for me, we should squash the commits and avoid merge commits (merge --ff-only).
@dberzano: We do not allow PR to the master, user can only PR to the dev branch.
best,
Mohammad

@dberzano

Copy link
Copy Markdown
Contributor

@MohammadAlTurany Yes of course, but I was referring to the source branch not to the destination one. I realize my comment was not so clear. If a user has her branch called dev and makes a PR to the dev branch upstream, the moment she wants to sync with upstream a git reset is mandatory and git pull can't be done. This is not a big deal IMHO and I keep preferring squash merges.

@ktf
ktf merged commit e0c32a6 into AliceO2Group:dev Aug 19, 2016
@ktf

ktf commented Aug 19, 2016

Copy link
Copy Markdown
Member

Ok, since no one else complained I merged this.

knopers8 pushed a commit to knopers8/AliceO2 that referenced this pull request Sep 7, 2020
EmilGorm pushed a commit to EmilGorm/AliceO2 that referenced this pull request Nov 22, 2021
…roup#145)

* Extend Xicc pT to 36 and change rapidity binning in TH3F

* Implement d0xd0 variables for Xicc

* Fixing clang format

* Fixing column and label ordering

* Adding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants